Move `RemoveResourceDesigner` trimmer step to post-trim pipeline by sbomer · Pull Request #10977 · dotnet/android

sbomer · 2026-03-19T21:36:10Z

Migrate RemoveResourceDesignerStep from an ILLink custom step to a
post-trimming IAssemblyModifierPipelineStep, following the same pattern
used for StripEmbeddedLibrariesStep and PostTrimmingAddKeepAlivesStep.

Add PostTrimmingRemoveResourceDesignerStep with self-contained logic
from LinkDesignerBase/RemoveResourceDesignerStep, using Action
for logging instead of ILLink's Context.LogMessage
Add AndroidLinkResources property to PostTrimmingPipeline task and
pre-load all assemblies when enabled (the step needs a two-phase scan)
Remove RemoveResourceDesignerStep and GetAssembliesStep from ILLink
custom steps in targets
Remove AndroidLinkConfiguration.cs and RemoveResourceDesignerStep.cs
from ILLink csproj (no longer needed there)
Delete GetAssembliesStep.cs (dead code, only served RemoveResourceDesignerStep)

Migrate AddKeepAlivesStep out of the ILLink custom step pipeline into a standalone MSBuild task that runs AfterTargets="ILLink", following the same pattern established by StripEmbeddedLibraries in #10894. Core IL-rewriting logic is extracted into AddKeepAlivesHelper, shared by both the new task (trimmed builds) and the existing pipeline step (non-trimmed builds via LinkAssembliesNoShrink).

Replace DefaultAssemblyResolver with DirectoryAssemblyResolver (ReadWrite=true) to avoid file handle conflicts on Windows. The directory resolver caches all assemblies (both explicit and dependency-resolved), preventing duplicate file opens that caused IOException when the same assembly was opened as both a dependency and a primary target.

Replace standalone AddKeepAlives and StripEmbeddedLibraries MSBuild tasks with a single PostTrimmingPipeline task that opens assemblies once (via DirectoryAssemblyResolver with ReadWrite) and runs both modifications in a single pass. Extract StripEmbeddedLibrariesStep as an IAssemblyModifierPipelineStep for reuse.

…Step pattern Use List<IAssemblyModifierPipelineStep> with StripEmbeddedLibrariesStep and PostTrimmingAddKeepAlivesStep instead of calling helpers directly. Move the IsFrameworkAssembly check into StripEmbeddedLibrariesStep.ProcessAssembly and remove all outer-loop filtering so each step handles its own guards internally.

…line Migrate RemoveResourceDesignerStep from an ILLink custom step to a post-trimming IAssemblyModifierPipelineStep, following the same pattern used for StripEmbeddedLibrariesStep and PostTrimmingAddKeepAlivesStep. - Add PostTrimmingRemoveResourceDesignerStep with self-contained logic from LinkDesignerBase/RemoveResourceDesignerStep, using Action<string> for logging instead of ILLink's Context.LogMessage - Add AndroidLinkResources property to PostTrimmingPipeline task and pre-load all assemblies when enabled (the step needs a two-phase scan) - Remove RemoveResourceDesignerStep and GetAssembliesStep from ILLink custom steps in targets - Remove AndroidLinkConfiguration.cs and RemoveResourceDesignerStep.cs from ILLink csproj (no longer needed there) - Delete GetAssembliesStep.cs (dead code, only served RemoveResourceDesignerStep)

…eDesignerStep The old ILLink RemoveResourceDesignerStep is no longer compiled into Xamarin.Android.Build.Tasks, so the PostTrimming prefix is unnecessary. Also remove AndroidLinkConfiguration.cs from compilation as it was only used by the old ILLink step.

Copilot

Pull request overview

Moves the RemoveResourceDesigner logic out of ILLink custom steps and into the post-trimming PostTrimmingPipeline, aligning it with other post-trim assembly modifiers in Xamarin.Android.Build.Tasks.

Changes:

Adds AndroidLinkResources support to PostTrimmingPipeline and runs RemoveResourceDesignerStep post-trim when enabled.
Removes RemoveResourceDesignerStep/GetAssembliesStep from ILLink custom steps wiring in Microsoft.Android.Sdk.TypeMap.LlvmIr.targets.
Stops compiling/removes ILLink-side sources (RemoveResourceDesignerStep link and GetAssembliesStep).

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
src/Xamarin.Android.Build.Tasks/Xamarin.Android.Build.Tasks.csproj	Updates compile includes for linker/tuner steps (and surfaces now-dead code).
src/Xamarin.Android.Build.Tasks/Tasks/PostTrimmingPipeline.cs	Adds `AndroidLinkResources` option and runs resource-designer removal post-trim.
src/Xamarin.Android.Build.Tasks/Microsoft.Android.Sdk/targets/Microsoft.Android.Sdk.TypeMap.LlvmIr.targets	Removes ILLink custom steps and passes `AndroidLinkResources` into the post-trim task.
src/Xamarin.Android.Build.Tasks/Linker/MonoDroid.Tuner/RemoveResourceDesignerStep.cs	Refactors step to run as an `IAssemblyModifierPipelineStep` with internal logging.
src/Microsoft.Android.Sdk.ILLink/Microsoft.Android.Sdk.ILLink.csproj	Stops linking `RemoveResourceDesignerStep.cs` into the ILLink project.
src/Microsoft.Android.Sdk.ILLink/GetAssembliesStep.cs	Deletes the now-dead ILLink step used to collect assemblies.

src/Xamarin.Android.Build.Tasks/Xamarin.Android.Build.Tasks.csproj

Copilot · 2026-03-24T22:15:03Z

src/Xamarin.Android.Build.Tasks/Linker/MonoDroid.Tuner/RemoveResourceDesignerStep.cs

+					designer = new TypeDefinition ("_Linker.Generated", "Resource", TypeAttributes.Public | TypeAttributes.AnsiClass);
+					designer.BaseType = new TypeDefinition ("System", "Object", TypeAttributes.Public | TypeAttributes.AnsiClass);


In the fallback path where no ResourceDesignerAttribute is found and an unresolved member reference is detected, this creates a new _Linker.Generated.Resource TypeDefinition but never adds it to any module’s Types collection, and sets BaseType to a new TypeDefinition instead of a TypeReference in the same module. As written, the log says the type is being added, but the assembly won’t actually change (and ProcessAssemblyDesigner will still report the assembly as modified). Add the generated type to assembly.MainModule.Types (or the appropriate module) and set BaseType via module.TypeSystem.Object/ImportReference, or avoid returning true unless the assembly is really mutated.

Suggested change

designer = new TypeDefinition ("_Linker.Generated", "Resource", TypeAttributes.Public | TypeAttributes.AnsiClass);

designer.BaseType = new TypeDefinition ("System", "Object", TypeAttributes.Public | TypeAttributes.AnsiClass);

var module = assembly.MainModule;

designer = new TypeDefinition ("_Linker.Generated", "Resource", TypeAttributes.Public | TypeAttributes.AnsiClass);

designer.BaseType = module.TypeSystem.Object;

module.Types.Add (designer);

Copilot · 2026-03-24T22:15:03Z

src/Xamarin.Android.Build.Tasks/Linker/MonoDroid.Tuner/RemoveResourceDesignerStep.cs

+				if (attribute.HasProperties)
+				{
+					foreach (var p in attribute.Properties)
+					{
+						if (p.Name == "IsApplication" && (bool)p.Argument.Value == (mainApplication ? mainApplication : (bool)p.Argument.Value))
+						{
+							designerFullName = attribute.ConstructorArguments[0].Value.ToString ();
+							break;
+						}
+					}
 				}


The IsApplication property check effectively becomes “match any value” when mainApplication is false ((bool)p.Argument.Value == (mainApplication ? mainApplication : (bool)p.Argument.Value) is always true in that case). That means non-main assemblies can incorrectly match an application designer attribute and then fail to locate the type, pushing the code into the member-ref fallback. Consider simplifying this to compare against mainApplication (or explicitly handle the ‘property missing’ case) so the correct designer is selected deterministically.

Suggested change

if (attribute.HasProperties)

{

foreach (var p in attribute.Properties)

{

if (p.Name == "IsApplication" && (bool)p.Argument.Value == (mainApplication ? mainApplication : (bool)p.Argument.Value))

{

designerFullName = attribute.ConstructorArguments[0].Value.ToString ();

break;

}

}

}

bool? isApplication = null;

if (attribute.HasProperties)

{

foreach (var p in attribute.Properties)

{

if (p.Name == "IsApplication")

{

isApplication = (bool)p.Argument.Value;

break;

}

}

}

bool matches;

if (isApplication.HasValue) {

matches = isApplication.Value == mainApplication;

} else {

// When IsApplication is not specified, treat the attribute as matching

// only for the main application assembly.

matches = mainApplication;

}

if (matches) {

designerFullName = attribute.ConstructorArguments[0].Value.ToString ();

}

Copilot · 2026-03-24T22:15:04Z

src/Xamarin.Android.Build.Tasks/Linker/MonoDroid.Tuner/RemoveResourceDesignerStep.cs

-				assembly.CustomAttributes.Remove (designerAttribute);
+	void FixUpdateIdValuesBody (MethodDefinition method)
+	{
+		List<Instruction> finalInstructions = new List<Instruction> ();


FixUpdateIdValuesBody allocates finalInstructions but never uses it. Removing the unused variable will keep this method clearer and avoid dead-code warnings if analyzers are enabled.

Suggested change

List<Instruction> finalInstructions = new List<Instruction> ();

jonathanpeppers

🤖 AI Review Summary

Verdict: ⚠️ Needs Changes

Found 3 issues: 1 warning, 2 suggestions

The overall migration from ILLink custom step to post-trimming IAssemblyModifierPipelineStep is well-structured and follows the established patterns (StripEmbeddedLibrariesStep, PostTrimmingAddKeepAlivesStep). CI is green.

⚠️ Code duplication: ~130 lines of Cecil manipulation code copied verbatim from LinkDesignerBase (RemoveResourceDesignerStep.cs:95)
💡 Dead code: Unused variable finalInstructions in FixUpdateIdValuesBody (RemoveResourceDesignerStep.cs:311)
💡 Dead code: Unused parameter module in RemoveFieldsFromType (RemoveResourceDesignerStep.cs:282)

👍 Nice touches:

Clean separation from ILLink dependencies
Good XML doc comment documenting the step's purpose and origin
Lazy LoadDesigner() with designerLoaded flag handles the two-phase scan correctly
Proper pre-loading of all assemblies via resolver.GetAssembly() (cached, so same instances used in the main loop)
GetAssembliesStep.cs deletion cleans up dead code

📝 Note: The PR description mentions "Remove AndroidLinkConfiguration.cs [...] from ILLink csproj" but the diff doesn't include that change. AndroidLinkConfiguration may now be dead code since its only consumers (GetAssembliesStep and the old ILLink RemoveResourceDesignerStep) are both removed from the ILLink pipeline.

Review generated by android-reviewer from review guidelines.

src/Xamarin.Android.Build.Tasks/Linker/MonoDroid.Tuner/RemoveResourceDesignerStep.cs

jonathanpeppers · 2026-03-30T15:28:17Z

src/Xamarin.Android.Build.Tasks/Linker/MonoDroid.Tuner/RemoveResourceDesignerStep.cs

-				assembly.CustomAttributes.Remove (designerAttribute);
+	void FixUpdateIdValuesBody (MethodDefinition method)
+	{
+		List<Instruction> finalInstructions = new List<Instruction> ();


🤖 💡 Code organization — finalInstructions is allocated but never used. This was dead code in the original LinkDesignerBase.FixUpdateIdValuesBody too — since you're rewriting the file, this is a good opportunity to remove it.

Rule: Remove unused code (Postmortem #58)

jonathanpeppers · 2026-03-30T15:28:17Z

src/Xamarin.Android.Build.Tasks/Linker/MonoDroid.Tuner/RemoveResourceDesignerStep.cs

+		}
+	}
+
+	void RemoveFieldsFromType (TypeDefinition type, ModuleDefinition module)


🤖 💡 Code organization — The module parameter is never used in this method body. Consider removing it (and updating the call site at line 226).

Rule: Remove unused code (Postmortem #58)

sbomer added 7 commits March 16, 2026 11:10

Fix missing using directive in AddKeepAlivesHelper.cs

e24ddc3

sbomer self-assigned this Mar 23, 2026

Merge origin/main into remove-resource-designer

c34d861

sbomer mentioned this pull request Mar 23, 2026

Migrate away from ILLink custom steps #10842

Open

8 tasks

sbomer marked this pull request as ready for review March 24, 2026 22:11

sbomer requested review from grendello, jonathanpeppers and simonrozsival as code owners March 24, 2026 22:11

Copilot AI review requested due to automatic review settings March 24, 2026 22:11

Copilot started reviewing on behalf of sbomer March 24, 2026 22:11 View session

Copilot AI reviewed Mar 24, 2026

View reviewed changes

sbomer mentioned this pull request Mar 27, 2026

Move RemoveResourceDesignerStep out of ILLink process #11039

Merged

jonathanpeppers reviewed Mar 30, 2026

View reviewed changes

jonathanpeppers approved these changes Mar 30, 2026

View reviewed changes

jonathanpeppers merged commit 0a5601a into main Mar 30, 2026
10 checks passed

jonathanpeppers deleted the dev/sbomer/remove-resource-designer branch March 30, 2026 15:30

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Move `RemoveResourceDesigner` trimmer step to post-trim pipeline#10977

Move `RemoveResourceDesigner` trimmer step to post-trim pipeline#10977
jonathanpeppers merged 8 commits intomainfrom
dev/sbomer/remove-resource-designer

sbomer commented Mar 19, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Copilot AI Mar 24, 2026

Uh oh!

Copilot AI Mar 24, 2026

Uh oh!

Copilot AI Mar 24, 2026

Uh oh!

jonathanpeppers left a comment

Uh oh!

Uh oh!

jonathanpeppers Mar 30, 2026

Uh oh!

jonathanpeppers Mar 30, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

		designer = new TypeDefinition ("_Linker.Generated", "Resource", TypeAttributes.Public \| TypeAttributes.AnsiClass);
		designer.BaseType = new TypeDefinition ("System", "Object", TypeAttributes.Public \| TypeAttributes.AnsiClass);

-				if (attribute.HasProperties)
-				{
-					foreach (var p in attribute.Properties)
-					{
-						if (p.Name == "IsApplication" && (bool)p.Argument.Value == (mainApplication ? mainApplication : (bool)p.Argument.Value))
-						{
-							designerFullName = attribute.ConstructorArguments[0].Value.ToString ();
-							break;
-						}
-					}
-				}
+				bool? isApplication = null;
+				if (attribute.HasProperties)
+				{
+					foreach (var p in attribute.Properties)
+					{
+						if (p.Name == "IsApplication")
+						{
+							isApplication = (bool)p.Argument.Value;
+							break;
+						}
+					}
+				}
+				bool matches;
+				if (isApplication.HasValue) {
+					matches = isApplication.Value == mainApplication;
+				} else {
+					// When IsApplication is not specified, treat the attribute as matching
+					// only for the main application assembly.
+					matches = mainApplication;
+				}
+				if (matches) {
+					designerFullName = attribute.ConstructorArguments[0].Value.ToString ();
+				}

Conversation

sbomer commented Mar 19, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Copilot AI Mar 24, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 24, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 24, 2026

Choose a reason for hiding this comment

Uh oh!

jonathanpeppers left a comment

Choose a reason for hiding this comment

🤖 AI Review Summary

Uh oh!

Uh oh!

jonathanpeppers Mar 30, 2026

Choose a reason for hiding this comment

Uh oh!

jonathanpeppers Mar 30, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants